Skip to content
This repository was archived by the owner on Feb 5, 2020. It is now read-only.

Ingress certs#1818

Closed
squat wants to merge 3 commits intocoreos:masterfrom
squat:ingress_certs
Closed

Ingress certs#1818
squat wants to merge 3 commits intocoreos:masterfrom
squat:ingress_certs

Conversation

@squat
Copy link
Contributor

@squat squat commented Sep 1, 2017

Enable user-provided signed certificates for ingress.
fixes INST-148
cc @s-urbaniak @alexsomesan @sym3tri @kyoto
2017-08-31-20 12 33

@squat squat closed this Sep 1, 2017
@squat squat reopened this Sep 1, 2017
@@ -0,0 +1,11 @@
output "ca_cert_pem" {
value = "${data.null_data_source.ingress_certs.inputs["ca_cert_pem"]}"
Copy link
Contributor Author

@squat squat Sep 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this value is actually needed anywhere but I added it to align with the API in #1811. We can always keep this just in case for future use.

caType === 'ca-signed' && <div>
<div className="row form-group">
<div className="col-xs-12">
<WithClusterConfig field={INGRESS_CERTIFICATE}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was told by @kans that WithClusterConfig is legacy and we should be switching over to Connect instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% agree. Im using it in this PR for consistency. We'll migrate this component over in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

- name: BRIDGE_LICENSE_FILE
value: /etc/tectonic/licenses/license
- name: BRIDGE_DEX_CLIENT_CA_FILE
value: /etc/tectonic-identity-grpc-client-secret/ca-cert
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the heads up, @s-urbaniak

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem 👍

@s-urbaniak
Copy link
Contributor

@sym3tri @squat just a logistical heads-up: As discussed OOB #1811 introduces TLS modules in a fashion that modules can be swapped manually, as described in modules/tls/ingress/user-provided/README.md.

This PR on the other hand uses ternaries and a null data source inside the ingress TLS module to switch module functionality (user-provided vs self-signed).

Once swappable module land (INST-143), #1811 can be integrated in the UI.

I'm leaving the final call to @sym3tri to decide which PR lands first. I am leaning towards #1811, because in conjunction with INST-143 it will be the cleaner approach from a TF perspective.

@squat
Copy link
Contributor Author

squat commented Sep 1, 2017

@s-urbaniak yes sounds good. If swappable modules (e.g. via symlinks rather than by hand) will land in the short term then we may be able to delay/replace this PR. Do we have a projection for when it will be ready? It would definitely be simpler/flatter from a TF point of view, though arguably more complex in another respect. It all comes down to how urgent ca-signed ingress certs in the GUI are. I tried to keep the API for this PR as close to #1811 as possible to facilitate a rebase on that branch. We could of course also merge a combination of the two and then add swappable modules down the line.

@s-urbaniak
Copy link
Contributor

@squat INST-143 is planned for the next sprint. I set the do-not-merge label tentatively.

@adarshaj
Copy link
Contributor

I am interested in this, I tried to manually overwrite the ingress tls secrets with letsencrypt certificates and locked myself out of dashboard, because login calls fail with invalid_code (-_-)! (I was seeing this error on the default nginx backend logs - OCSP_basic_verify() failed (SSL: error:27069076:OCSP routines:OCSP_basic_verify:signer certificate not found) with an HTTP error of 499). I hope this PR would make it possible to set CA for ingress and thereby not cause anyone to lock themselves out of dashboard!

@s-urbaniak
Copy link
Contributor

@adarshaj you can follow the instructions at https://github.com/coreos/tectonic-installer/tree/master/modules/tls/ingress/user-provided to set up user provided ingress certs. We are working on a way to automate this process.

@adarshaj
Copy link
Contributor

@s-urbaniak Thank you, I followed the steps there and got it to working state (on bare-metal)! 🎉

@sym3tri
Copy link
Contributor

sym3tri commented Dec 1, 2017

@squat I'm going to close this out for now since it's stale. Please keep your branch around & let's revisit once we have a swappable module solution.

@sym3tri sym3tri closed this Dec 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants